Skip to content

override pac version to v0.36.0 in dev/staging #7538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

aThorp96
Copy link
Contributor

@aThorp96 aThorp96 commented Aug 7, 2025

The nightly build should be using PaC v0.36.X, however some automation seems to have caused the nightly build to switch back to v0.35.X several days after it updated to v0.36.X.

This PR overrides the PaC images to those which point to v0.36.0

@openshift-ci openshift-ci bot requested review from enarha and Roming22 August 7, 2025 19:37
Copy link

openshift-ci bot commented Aug 7, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aThorp96
Once this PR has been reviewed and has the lgtm label, please assign roming22 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

github-actions bot commented Aug 7, 2025

Code Review by Gemini

The following is a review of the provided code changes, highlighting potential issues and suggesting improvements.

Summary of Changes:

This pull request involves significant refactoring and updates across various components, including:

  • Removal of Kueue from production environments: Many files related to Kueue deployment and configuration in production clusters have been deleted.
  • Reduced KubeArchive deployments: KubeArchive is being removed or disabled in several clusters, particularly in staging and some production environments.
  • Toolchain Operator Refactoring: The deployment strategy for toolchain-host-operator and toolchain-member-operator has been refactored, consolidating cluster-specific kustomizations into more generalized ones.
  • Component Version Updates: Numerous components like build-service, integration-service, internal-services, konflux-ui, mintmaker, multi-platform-controller, pulp-access-controller, and release-service are updated to newer versions (indicated by SHA changes).
  • Pipelines-as-Code (PaC) Version Override: Explicit overrides for PaC image versions to v0.36.0 are introduced in development and staging environments, aligning with the commit title.
  • Infrastructure Configuration Changes: Updates to multi-platform controller host configurations (e.g., IP addresses, platform types, removal of s390x hosts, GPU instance changes).
  • Monitoring Adjustments: Changes to Prometheus federation rules and Grafana dashboard sources.
  • Policy Updates: Removal of KubeArchive-related policies from certain clusters.
  • Documentation and Workflow Changes: Removal of the Gemini Code Review workflow, updates to konflux-info README and schema, and changes to OWNERS files.

Identified Issues and Suggestions:


1. File: components/konflux-kite/base/kustomization.yaml

  • Issue: The images section, which explicitly defines the newTag for quay.io/konflux-ci/kite-prod and quay.io/konflux-ci/kite-init, has been removed. This means the image versions will now be implicitly determined by the base Kustomization or other default mechanisms. Relying on implicit versions can lead to unexpected deployments if the base changes or if the desired version is not consistently applied.
  • Line(s) to change:
    --- a/components/konflux-kite/base/kustomization.yaml
    +++ b/components/konflux-kite/base/kustomization.yaml
    @@ -7,12 +7,6 @@
       - deployment.yaml
       - service.yaml
       - routes.yaml
     
     namespace: konflux-kite
     
    -images:
    -  - name: quay.io/konflux-ci/kite-prod
    -    newTag: ef9cf97e2949abd49b725958250b40c59b63b8c9
    -
    -  - name: quay.io/konflux-ci/kite-init
    -    newTag: ef9cf97e2949abd49b725958250b40c59b63b8c9
    -
     configMapGenerator:
       - name: kite-config
         namespace: konflux-kite
  • Suggestion: Re-add the images section with explicit newTag values to ensure deterministic and reproducible deployments of Konflux Kite.

2. Files: components/mintmaker/production/kflux-ocp-p01/kustomization.yaml, components/mintmaker/production/kflux-osp-p01/kustomization.yaml, components/mintmaker/production/kflux-prd-rh02/kustomization.yaml, components/mintmaker/production/kflux-prd-rh03/kustomization.yaml, components/mintmaker/production/kflux-rhel-p01/kustomization.yaml, components/mintmaker/production/stone-prod-p01/kustomization.yaml

  • Issue: Similar to the konflux-kite issue, the images section, which explicitly defines the newTag for quay.io/konflux-ci/mintmaker, has been removed from these cluster-specific Kustomization files. This can lead to non-deterministic image versions being deployed.
  • Line(s) to change (example for components/mintmaker/production/kflux-ocp-p01/kustomization.yaml):
    --- a/components/mintmaker/production/kflux-ocp-p01/kustomization.yaml
    +++ b/components/mintmaker/production/kflux-ocp-p01/kustomization.yaml
    @@ -1,21 +1,15 @@
     apiVersion: kustomize.config.k8s.io/v1beta1
     kind: Kustomization
     resources:
     - ../base
     namespace: mintmaker
    -
    -images:
    -  - name: quay.io/konflux-ci/mintmaker
    -    newName: quay.io/konflux-ci/mintmaker
    -    newTag: a3816e76f5e06ba3114731fc34fa14f76d442b6c
    -
     patches:
       - path: pipelines-as-code-secret-path.yaml
         target:
           name: pipelines-as-code-secret
           group: external-secrets.io
           version: v1beta1
           kind: ExternalSecret
     
     components:
  • Suggestion: Re-add the images section with explicit newTag values to ensure deterministic and reproducible deployments of Mintmaker across these production clusters.

3. File: components/release/production/kustomization.yaml

  • Issue: There's an inconsistency in the SHA references for the release-service. The ref for the base configuration is updated to 48d3044042ac90957d64850c85c355832a82173b, but the newTag for the image is updated to 054fd6de76eadc41ef28942d8fcc3eb37bb9438b. While it's possible for the configuration and image to have different SHAs, it's often a source of confusion or potential mismatch if they are expected to be from the same release or commit.
  • Line(s) to change:
    --- a/components/release/production/kustomization.yaml
    +++ b/components/release/production/kustomization.yaml
    @@ -1,17 +1,17 @@
     apiVersion: kustomize.config.k8s.io/v1beta1
     kind: Kustomization
     resources:
       - ../base
       - ../base/monitor/production
    -  - https://github.com/konflux-ci/release-service/config/default?ref=26f6f47a8e09862c907f5ae1e820fc319e0c2866
    +  - https://github.com/konflux-ci/release-service/config/default?ref=48d3044042ac90957d64850c85c355832a82173b
       - release_service_config.yaml
     
     components:
       - ../k-components/manager-resources-patch
     
     images:
       - name: quay.io/konflux-ci/release-service
         newName: quay.io/konflux-ci/release-service
    -    newTag: 26f6f47a8e09862c907f5ae1e820fc319e0c2866
    +    newTag: 054fd6de76eadc41ef28942d8fcc3eb37bb9438b
     
     namespace: release-service
  • Suggestion: Verify if the ref and newTag are intended to be different. If they should be synchronized, update one to match the other. If they are intentionally different, consider adding a comment explaining why, to improve clarity for future maintainers.

4. File: hack/kueue-vm-quotas/update-kueue-vm-quotas.py

  • Issue 1: The change removes sorted(data.items()) when iterating through the host configuration. While Python 3.7+ dictionaries preserve insertion order, relying on this for configuration processing that might be sensitive to order (e.g., how resource groups or flavors are generated) can be less robust and harder to debug if the order ever changes unexpectedly (e.g., due to different Python versions or dictionary implementations in other contexts). The original comment about "deterministic processing order" was important.

  • Line(s) to change:

    --- a/hack/kueue-vm-quotas/update-kueue-vm-quotas.py
    +++ b/hack/kueue-vm-quotas/update-kueue-vm-quotas.py
    @@ -103,8 +103,7 @@
     def parse_host_config(host_config_path: str) -> Dict[str, PlatformQuota]:
         """Parse host-config.yaml and extract platform quotas."""
         with open(host_config_path, 'r') as f:
             config = yaml.safe_load(f)
         
         platform_quotas: Dict[str, PlatformQuota] = {}
         data = config.get('data', {})
    -    
    -    # Sort keys for deterministic processing order
    -    for key, value in sorted(data.items()):
    +    for key, value in data.items():
             platform = None
             
             # Process dynamic platforms
  • Suggestion: Revert the change to for key, value in sorted(data.items()): to ensure deterministic processing order, which is crucial for configuration generation scripts.

  • Issue 2: The yaml.dump_all function is now called with sort_keys=False. This means the generated YAML output will not have its keys sorted alphabetically. While not a functional bug, unsorted YAML can make diffs harder to read and review, especially for large configuration files, and can lead to unnecessary noise in version control.

  • Line(s) to change:

    --- a/hack/kueue-vm-quotas/update-kueue-vm-quotas.py
    +++ b/hack/kueue-vm-quotas/update-kueue-vm-quotas.py
    @@ -276,11 +275,9 @@
         print(f"Created resource group {group_index} with {len(platforms)} platforms: {', '.join(platform_names)}")
     
     # Validate and update
     validate_constraints(new_resource_groups)
     cluster_queue_doc['spec']['resourceGroups'] = new_resource_groups
     
    -    # Sort documents for consistent ordering
    -    documents.sort(key=lambda doc: (
    -        doc.get('kind', ''),
    -        doc.get('metadata', {}).get('name', '') if doc.get('metadata') else ''
    -    ))
    -    
    -    # Write back with sorted keys for deterministic output
     with open(cluster_queue_path, 'w') as f:
    -        yaml.dump_all(documents, f, default_flow_style=False, sort_keys=True)
    +        yaml.dump_all(documents, f, default_flow_style=False, sort_keys=False)
     
     print(f"\nUpdated {cluster_queue_path}")
     print_summary(new_resource_groups, len(platform_quotas))
  • Suggestion: Change sort_keys=False back to sort_keys=True in yaml.dump_all to ensure consistent and readable YAML output.


@aThorp96
Copy link
Contributor Author

aThorp96 commented Aug 7, 2025

/hold

There is currently a custom PaC build deployed to staging for Kueue testing

Copy link
Contributor

github-actions bot commented Aug 8, 2025

Code Review by Gemini

## Code Review

### `components/pipeline-service/development/main-pipeline-service-configuration.yaml`

*   **Improvement:** The `TODO` comment is well-placed and indicates the temporary nature of the override. No issues found.

### `components/pipeline-service/staging/base/main-pipeline-service-configuration.yaml`

*   **Improvement:** The commented-out block for `IMAGE_PAC_PAC_CONTROLLER`, `IMAGE_PAC_PAC_WATCHER`, and `IMAGE_PAC_PAC_WEBHOOK` (lines 2098-2106 in the new file) adds clutter. If these images are no longer actively used or are for a very specific, temporary testing scenario, consider removing this commented-out section to keep the configuration clean.

    ```diff
    --- a/components/pipeline-service/staging/base/main-pipeline-service-configuration.yaml
    +++ b/components/pipeline-service/staging/base/main-pipeline-service-configuration.yaml
    @@ -2095,11 +2095,6 @@
     +      - name: IMAGE_PAC_PAC_WATCHER
     +        value: quay.io/openshift-pipeline/pipelines-pipelines-as-code-watcher-rhel9@sha256:5bdbc16185dc0def4f9bb1d1b67f5bd83363f8c4285d91597641099322d2a030
     +      # TODO: images used for Kueue testing @gbenhaim
     +      # - name: IMAGE_PAC_PAC_CONTROLLER
     +      #   value: ghcr.io/gbenhaim/pipelines-as-code/pipelines-as-code-controller@sha256:dea65f29afcecdd837b0b7389c6824d546b5098bf98ba98f02f27d6a85b0b376
     +      # - name: IMAGE_PAC_PAC_WATCHER
     +      #   value: ghcr.io/gbenhaim/pipelines-as-code/pipelines-as-code-watcher@sha256:09964eff749f2fb620ae1500cfc538d0765cf0e29fb2f177abb3521400fad403
     +      # - name: IMAGE_PAC_PAC_WEBHOOK
     +      #   value: ghcr.io/gbenhaim/pipelines-as-code/pipelines-as-code-webhook@sha256:440d6f9311d23b994ef68ffe0513eba832b891fdfb6360385bb635ba154f7526
     ```

### `components/pipeline-service/staging/stone-stage-p01/deploy.yaml`

*   No issues found. The image overrides are consistent with the intended change.

### `components/pipeline-service/staging/stone-stg-rh01/deploy.yaml`

*   No issues found. The image overrides are consistent with the intended change.

@aThorp96
Copy link
Contributor Author

Closing in favor of #7619

@aThorp96 aThorp96 closed this Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants